Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve multi-threaded UAF analysis and add SVComp result generation #1123

Merged
merged 24 commits into from
Sep 29, 2023

Conversation

mrstanb
Copy link
Member

@mrstanb mrstanb commented Jul 31, 2023

This PR:

  • Updates the multi-threaded UAF analysis by now globally tracking a set of tuples of the form (tid, {must-joined threads}), where tid denotes the tid of a thread which has freed the memory being tracked for UAF
  • Adds SVComp result generation for UAF-related memory safety bugs

@sim642 sim642 added this to the SV-COMP 2024 milestone Jul 31, 2023
@sim642 sim642 added the sv-comp SV-COMP (analyses, results), witnesses label Jul 31, 2023
src/analyses/useAfterFree.ml Outdated Show resolved Hide resolved
src/witness/svcomp.ml Outdated Show resolved Hide resolved
src/analyses/useAfterFree.ml Outdated Show resolved Hide resolved
@sim642 sim642 self-requested a review August 16, 2023 09:08
Comment on lines +30 to +39
else if Str.string_match regexp s 0 then
let global = Str.matched_group 1 s in
if global = "valid-free" then
ValidFree
else if global = "valid-deref" then
ValidDeref
else if global = "valid-memtrack" then
ValidMemtrack
else
failwith "Svcomp.Specification.of_string: unknown global expression"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is that the property file in SV-COMP doesn't contain just one, but all three at once: https://gitlab.com/sosy-lab/benchmarking/sv-benchmarks/-/blob/3d65c76d8521ef5bc79077a31e7b7e41dd077309/c/properties/valid-memsafety.prp.
But we can leave it for now if the idea is to at least test one of them specifically. I can do larger refactoring of the SV-COMP stuff to allow for multi-property.

It just means that as-is, this isn't ready for valid-memsafety.prp from SV-COMP.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest that we discuss and implement support for supporting all three properties, so that we can try and let all relevant analyses for memory safety run together in order to be able to evaluate how we are actually performing in SV-COMP for memory safety.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff was run on SV-COMP but that was done per-specification or? because there's no handling of all three simultaneously here still.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added a workaround by having an internal "memory-safety" property which unites all three. This way we were able to let all three run and be tested simultaneously for the benchmarks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, this will be fixed in a follow-up PR, which will contain commits from the https://github.com/goblint/analyzer/tree/staging_memsafety branch, we just don't want to include them here yet to avoid polluting the history.

src/framework/analysisState.ml Outdated Show resolved Hide resolved
Copy link
Member

@michael-schwarz michael-schwarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sim642 sim642 self-requested a review September 26, 2023 15:43
@michael-schwarz
Copy link
Member

@sim642 If you still want to review this, it would be nice to do it soon, we hope to merge most of Stanimir's things before he defends his thesis in two weeks.

src/analyses/useAfterFree.ml Outdated Show resolved Hide resolved
Comment on lines +30 to +39
else if Str.string_match regexp s 0 then
let global = Str.matched_group 1 s in
if global = "valid-free" then
ValidFree
else if global = "valid-deref" then
ValidDeref
else if global = "valid-memtrack" then
ValidMemtrack
else
failwith "Svcomp.Specification.of_string: unknown global expression"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff was run on SV-COMP but that was done per-specification or? because there's no handling of all three simultaneously here still.

@michael-schwarz
Copy link
Member

@mrstanb can you resolve the merge conflict with master? Then we should be good to go!

@mrstanb
Copy link
Member Author

mrstanb commented Sep 29, 2023

@mrstanb can you resolve the merge conflict with master? Then we should be good to go!

Sure, right away.

@michael-schwarz michael-schwarz merged commit 67160fe into goblint:master Sep 29, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
student-job sv-comp SV-COMP (analyses, results), witnesses
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants